Feature: Restart as admin & use New-NetNeighbor / Remove-NetNeighbor#3403
Merged
mergify[bot] merged 12 commits intomainfrom May 3, 2026
Merged
Feature: Restart as admin & use New-NetNeighbor / Remove-NetNeighbor#3403mergify[bot] merged 12 commits intomainfrom
mergify[bot] merged 12 commits intomainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the ARP Table feature to better align with the “admin-gated modifications” UX pattern used elsewhere (e.g., Firewall), and moves ARP write operations into an in-process PowerShell runspace with a shared lock.
Changes:
- Add admin-only gating for ARP table modification commands, including a “Restart as admin” command and a read-only banner when not elevated.
- Refactor ARP modification operations to use a shared PowerShell
RunspaceandSemaphoreSlimfor serialized execution; remove the legacy UAC-cancel event flow. - Add a new localized string for the ARP admin read-only message; remove an unused using directive.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Source/NETworkManager/Views/FirewallView.xaml.cs | Removes an unused System.Windows.Input using. |
| Source/NETworkManager/Views/ARPTableView.xaml | Adds a non-admin banner and “Restart as admin” button; wires visibility to ConfigurationManager.Current.IsAdmin. |
| Source/NETworkManager/ViewModels/ARPTableViewModel.cs | Introduces IsModifying, gates modify commands behind admin + in-flight flags, and adds RestartAsAdminCommand. |
| Source/NETworkManager.Models/Network/ARP.cs | Adds shared PowerShell runspace + lock; converts write ops to static async methods executed in-process. |
| Source/NETworkManager.Localization/Resources/Strings.resx | Adds ARPTableAdminMessage string. |
| Source/NETworkManager.Localization/Resources/Strings.Designer.cs | Adds the strongly-typed accessor for ARPTableAdminMessage. |
Files not reviewed (1)
- Source/NETworkManager.Localization/Resources/Strings.Designer.cs: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 42 out of 52 changed files in this pull request and generated 7 comments.
Files not reviewed (1)
- Source/NETworkManager.Localization/Resources/Strings.Designer.cs: Language not supported
Comments suppressed due to low confidence (1)
Source/NETworkManager/ViewModels/NeighborTableViewModel.cs:374
- AddEntryAction sets IsModifying=true and then awaits GetInterfacesAsync / shows the child window without a surrounding try/finally. If GetInterfacesAsync or ShowChildWindowAsync throws, IsModifying will remain true and the view will stay stuck with modify/refresh commands disabled. Wrap this method body in try/catch/finally (and surface an error message) so IsModifying is always reset on failure.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 42 out of 52 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- Source/NETworkManager.Localization/Resources/Strings.Designer.cs: Language not supported
Comments suppressed due to low confidence (1)
Source/NETworkManager/ViewModels/NeighborTableViewModel.cs:336
AddEntryAction()setsIsModifying = truebefore awaitingNeighborTable.GetInterfacesAsync(), but there’s notry/finallyaround that await. IfGetInterfacesAsync()throws (e.g., NetworkInterface APIs fail),IsModifyingwill remaintrueand permanently disable refresh/modify commands for the view. Consider movingIsModifying = trueto after the interface list is loaded, or wrapping the whole method intry/catch/finallythat always resetsIsModifyingon failure (and shows an error message).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+78
to
+85
| /// <summary> | ||
| /// Ensures that only one PowerShell pipeline runs on <see cref="SharedRunspace"/> at a time. | ||
| /// </summary> | ||
| private static readonly SemaphoreSlim Lock = new(1, 1); | ||
|
|
||
| /// <summary>Protects reads and writes to <see cref="_interfaceAliasCache"/> and <see cref="_interfaceAliasCacheExpiry"/>.</summary> | ||
| private static readonly Lock InterfaceAliasCacheLock = new(); | ||
|
|
…oot/NETworkManager into feature/neighbor-table
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes proposed in this pull request
This pull request implements a comprehensive rename and refactor of the application's "ARP Table" feature, transitioning it to "Neighbor Table" to better reflect support for both IPv4 ARP and IPv6 NDP. It updates code, resources, and documentation to use the new terminology throughout, while maintaining backward compatibility for legacy settings. Additionally, it introduces localization support for neighbor states and improves resource naming consistency.
Major terminology and feature refactor:
Renamed the
ARPTableapplication and related documentation identifiers toNeighborTable, updating all code references, resource strings, and documentation links accordingly. The obsoleteARPTableenum value is retained (marked[Obsolete]) for backward compatibility with old settings files. [1] [2] [3] [4] [5] [6] [7] [8]Updated resource files and localization to replace "ARP Table" with "Neighbor Table", and added new localized strings for neighbor states and related messages. [1] [2]
Localization and converter enhancements:
NeighborStateToStringConverterto convertNeighborStateenum values to localized strings, and registered a newNeighborStateresource identifier for translation. [1] [2]Resource and naming consistency improvements:
GroupDomainNametoGroupDomainfor clarity and consistency. [1] [2]Documentation and project structure:
AGENTS.mdto the root project guidelines and conventions.Versioning:
2026.5.3.0.Related issue(s)
To-Do
Contributing
By submitting this pull request, I confirm the following: